Fix several asm! related issues #94169
Conversation
|
Some changes occured to rustc_codegen_cranelift cc @bjorn3 |
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
| /// inline assembly. | ||
| fn asm_target_features<'tcx>(tcx: TyCtxt<'tcx>, id: DefId) -> &'tcx FxHashSet<Symbol> { | ||
| let mut target_features = tcx.sess.target_features.clone(); | ||
| let attrs = tcx.codegen_fn_attrs(id); |
There was a problem hiding this comment.
I think one problem that this has is that -Ctarget-feature flag attributes are not accounted for here, to the best of my knowledge. This is something we should eventually fix (at the very least because #[cfg(target_feature=...)] honour the global flags.) One of the commits in #87402 gets us partway towards a solution there by exposing the globally enabled features as a query, but those are still backend-specific names so not applicable for assembly quite yet.
|
Overall seems like an improvement. I'm happy to r+ this with or without the 2nd comment being addressed. The 1st comment is mostly just an observation and something to keep in mind, as it is not necessarily something we can easily fix right now anyway. |
|
@bors r+ |
|
📌 Commit f10f921 has been approved by |
bors
added
S-waiting-on-bors
Fix several asm! related issues This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other. Fixes rust-lang#92378 Fixes rust-lang#85247 r? `@nagisa`
Fix several asm! related issues This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other. Fixes rust-lang#92378 Fixes rust-lang#85247 r? ``@nagisa``
https://github.com/rust-lang-ci/rust/runs/5267509172?check_suite_focus=true |
|
@bors r- |
bors
added
S-waiting-on-author
This is already handled by supported_types().
The previous approach of checking for the reserve-r9 target feature didn't actually work because LLVM only sets this feature very late when initializing the per-function subtarget.
Checking of asm! register operands now properly takes function attributes such as #[target_feature] and #[instruction_set] into account.
|
I'm not sure how I should deal with this, should I just add |
|
The alternative would be to move the query implementation to somewhere else (maybe But yeah, adding the comment is something you can do. In fact this comment was present in this file before bb0a2f9. |
|
@bors r=nagisa |
|
📌 Commit a60b791 has been approved by |
bors
added
S-waiting-on-bors
Fix several asm! related issues This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other. Fixes rust-lang#92378 Fixes rust-lang#85247 r? `@nagisa`
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#94169 (Fix several asm! related issues) - rust-lang#94178 (tidy: fire less "ignoring file length unneccessarily" warnings) - rust-lang#94179 (solarish current_exe using libc call directly) - rust-lang#94196 (compiletest: Print process output info with less whitespace) - rust-lang#94208 (Add the let else tests found missing in the stabilization report) - rust-lang#94237 (Do not suggest wrapping an item if it has ambiguous un-imported methods) - rust-lang#94246 (ScalarMaybeUninit is explicitly hexadecimal in its formatting) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fix several asm! related issues This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other. Fixes rust-lang#92378 Fixes rust-lang#85247 r? ``@nagisa``
This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other.
Fixes #92378
Fixes #85247
r? @nagisa